Skip to content

Conversation

@justinc1
Copy link
Collaborator

Draft: this requires #187 to be merged first - only last commit in refactor-NicType branch is relevant.

The Nic._handle_nic_type method implemented conversion of NIC type from hypercore to ansible.
But in code it was used also to "convert" from ansible to ansible - whis was wrong.
Unittest did pass because we had also wrong unittest data - mock data pretended HyperCore returns "virtio", but in reality "VIRTIO" is returned.

This is now changed, unittest mock data is fixed, and Nic._handle_nic_type is replaced with NicType.hypercore_to_ansible.

RTL8139 = "RTL8139"
VIRTIO = "VIRTIO"
INTEL_E1000 = "INTEL_E1000"
class NicType:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just move this class to module_utils/nic.py (and delete type.py). But this is just my opinion, not necessary to change that. Otherwise nice job, I approve :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, will be done.

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
@justinc1 justinc1 marked this pull request as ready for review March 30, 2023 12:52
@justinc1 justinc1 merged commit e52077e into main Mar 30, 2023
@justinc1 justinc1 deleted the refactor-NicType branch March 30, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants